-
Notifications
You must be signed in to change notification settings - Fork 12
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Multischema endpoint #158
Multischema endpoint #158
Conversation
If some caution is needed perhaps it’s time we added some tests for this |
Agreed, will add tests before this gets merged. Although what I was concerned about should be caught by the materialization client tests, if I understand them correctly. |
Tests added for the table manager. Not perfect because they have to mock the metadata one gets from the server, but at least there's an example there to compare against. |
tests/test_tablemanager.py
Outdated
assert 'allen_column_mtypes_v2' == qry.joins_kwargs.get('joins')[0][0] | ||
|
||
def test_view_manager(): | ||
views = ViewManager(None, view_metadata=views_list, view_schema=views_schema) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this should be done with responses so it tests that portion of the code
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fair, changed now.
Adding the multischema download endpoints and using them in the tables/views interface. Wound up refactoring the materialization initialization to use multithreading for pinging the server for various information, but on the plus side the .tables and .views interfaces are populated at the same time with minor overhead cost. Also, added small tweaks that seem to improve tab-autocompletion for these services as well.
I think this is working, but there are some changes to client.materialize that could be significant if they don't work as expected, so a bit of caution is warranted.